Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

FlxObject: fix overlapsPoint() if overlap is at x or y = 0 #1818

Merged
merged 3 commits into from
Apr 21, 2016

Conversation

IBwWG
Copy link
Contributor

@IBwWG IBwWG commented Apr 20, 2016

The top and left edges of checked objects were not registering as overlapping, when they should be. (Consider the example of a single pixel at 0,0: the former could would never register an overlap with the mouse, always looking for your position to be > 0 and < 1. Even though it's a float, it still gets updated to integer values when you move the mouse.)

May seem trivial, but when you have a button at the edge of a full-screen game, it becomes important.

…rlapping, when they should be. (Consider the example of a single pixel at 0,0: the former could would never register an overlap with the mouse, always looking for your position to be > 0 and < 1. Even though it's a float, it still gets updated to integer values when you move the mouse.)

May seem trivial, but when you have a button at the edge of a full-screen game, it becomes important.
@Gama11
Copy link
Member

Gama11 commented Apr 20, 2016

This seems to be causing a failure in FlxButtonTest on neko and cpp:

FAIL: massive.munit.AssertionException: Value [1] was not equal to expected value [0] at flixel.ui.FlxButtonTest#testTriggerAnimationOnce (77)

@IBwWG
Copy link
Contributor Author

IBwWG commented Apr 20, 2016

Thanks, I'll have a look.

@IBwWG
Copy link
Contributor Author

IBwWG commented Apr 20, 2016

That's probably expected behaviour, if the mouse cursor is at 0,0 during the test, and the button is at 0,0. :)

@Gama11
Copy link
Member

Gama11 commented Apr 20, 2016

I just checked this with a screen magnifier and a FlxButton - wow, how did nobody notice this before? This code has been around since the AS3 days. :)

Before mering though - this seems like the sort of thing that we should really have a unit test for, and that should also be pretty easy to test.

@Gama11 Gama11 added the Bug label Apr 20, 2016
…emove from the state afterwards so we don't double-add, while leaving the existing tests independent of having been add()ed.
@IBwWG
Copy link
Contributor Author

IBwWG commented Apr 21, 2016

I had noticed, because I had a bunch of buttons at the top of the screen that testers were complaining did not function well. I had thought I was somehow not understanding hitboxes correctly or something. When I finally dug into it I was thinking the same thing ;) Thanks for verifying...talk about being under the magnifying glass... :)

Hopefully this test is sufficient.

@IBwWG
Copy link
Contributor Author

IBwWG commented Apr 21, 2016

Super, it passed on Travis too, not just locally. (You never know ;) )

@Gama11 Gama11 changed the title Fix top/left edges not overlapping when they should FlxObject: fix overlapsPoint() if overlap is at x or y = 0 Apr 21, 2016
@Gama11 Gama11 merged commit 6395483 into HaxeFlixel:dev Apr 21, 2016
@IBwWG IBwWG deleted the fix-overlap-check branch April 21, 2016 11:50
Aurel300 pushed a commit to larsiusprime/haxeflixel that referenced this pull request Apr 18, 2018
…l#1818)

* The top and left edges of checked objects were not registering as overlapping, when they should be.  (Consider the example of a single pixel at 0,0: the former could would never register an overlap with the mouse, always looking for your position to be > 0 and < 1.  Even though it's a float, it still gets updated to integer values when you move the mouse.)

May seem trivial, but when you have a button at the edge of a full-screen game, it becomes important.

* Fix test by moving button out from under the cursor.

* Add unit test for HaxeFlixel#1818.  Also make the existing unit test remove from the state afterwards so we don't double-add, while leaving the existing tests independent of having been add()ed.
Aurel300 pushed a commit to larsiusprime/haxeflixel that referenced this pull request Apr 18, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants